Conversation
typescript-bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
typescript-bot
added
For Backlog Bug
@microsoft-github-policy-service agree |
|
@typescript-bot I saw that CI/ format check failed. I just formatted the file locally and I can push it up and add it to the pull request after the request has been reviewed. |
|
FWIW you do not need tests for this kind of change; it's honestly worse to write tests for these comments because updating them later will mean having to update tests to just copy the same text a second time. |
|
Thanks @jakebailey, I understand. I can take out my 2 tests if necessary. Also, I just pushed up 3 baseline/reference test files that had failed after reformatting. |
src/lib/es5.d.ts
Outdated
| * @param deleteCount The number of elements to remove. | ||
| * @param deleteCount The number of elements to remove. Omitting this argument will remove all elements from the start | ||
| * paramater location to end of the array. If value of this argument is either a negative number, zero, undefined, or a type | ||
| * that cannot be coverted to an integer, the function will evaluate the argument as zero and not remove any elements. |
There was a problem hiding this comment.
Hi @birgersp, yes it is a typo. I just fixed it and pushed it up.
src/lib/es5.d.ts
Outdated
| * @param deleteCount The number of elements to remove. If value of this argument is either a negative number, zero, | ||
| * undefined, or a type that cannot be coverted to an integer, the function will evaluate the argument as zero and | ||
| * not remove any elements. | ||
| * Note: If the deleteCount argument is left empty between the start and items arguments, it will not be evaluated as |
There was a problem hiding this comment.
what does "left empty" mean, specifically?
if you mean "omitted" then that's not really valid, because the compiler will throw an error; "No overload matches this call". IMO this note can be removed.
There was a problem hiding this comment.
@birgersp, yes I meant omitted. I just took out that sentence (that started with "Note") and pushed it up in the latest commit.
jakebailey
left a comment
jakebailey
left a comment
There was a problem hiding this comment.
There's no need for any of the changes in this PR other than the one in es5.d.ts. We typically do not create tests for simple doc changes.
github-project-automation
bot
moved this from Not started
to Waiting on author
in PR Backlog
|
Ha, I didn't notice my own comment from before; at least I'm consistent. But, please do remove all changes except those in |
|
Thanks, @jakebailey. I just removed everything except for es5.d.ts and 2 fourslash test baseline files that the doc changes affected. |
github-project-automation
bot
moved this from Waiting on author
to Needs merge
in PR Backlog
4 participants
Fixes #61081